Add a MIR pass manager, take 2 #91386
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
rust-highfive
added
the
S-waiting-on-review
|
r? @ghost |
ecstatic-morse
force-pushed
the
mir-pass-manager2
branch
from
6c3b0ce
to
5a1227d
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
bjorn3
reviewed
Nov 30, 2021
| } | ||
|
|
||
| tcx.sess.mir_opt_level() >= 3 | ||
| tcx.sess.opts.debugging_opts.inline_mir && tcx.sess.mir_opt_level() >= 3 |
There was a problem hiding this comment.
This changes the condition. Before it would always run if -Zinline-mir=yes is used, never if -Zinline-mir=no is used and only for mir opt level >= 3 if -Zinline-mir is not specified. After it will only run when -Zinline-mir=yes and mir opt level >= 3 is combined.
There was a problem hiding this comment.
Ah. I'm sorry. I understand the Option<bool> now.
|
☔ The latest upstream changes (presumably #91469) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Closing in favor of the more minimal #91475. I'd like to explore some of these ideas in the future, though. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 5, 2021
…li-obk Add a MIR pass manager (Taylor's Version) The final draft of rust-lang#91386 and rust-lang#77665. While the compile-time constraints in rust-lang#91386 are cool, I decided on a more minimal approach for now. I want to explore phase constraints and maybe relative-ordering constraints in the future, though. This should preserve existing behavior **exactly** (please let me know if it doesn't) while making the following changes to the way we organize things today: - Each `MirPhase` now corresponds to a single MIR pass. `run_passes` is not responsible for listing the correct MIR phase. - `run_passes` no longer silently skips passes if the declared MIR phase is greater than or equal to the body's. This has bitten me multiple times. If you want this behavior, you can always branch on `body.phase` yourself. - If your pass is solely to emit errors, you can use the `MirLint` interface instead, which gets a shared reference to `Body` instead of a mutable one. By differentiating the two, I hope to make it clearer in the short term where lints belong in the pipeline. In the long term perhaps we could enforce this at compile-time? - MIR is no longer dumped for passes that aren't enabled, or for lints. I tried to check that `-Zvalidate` still works correctly, since the MIR phase is now updated as soon as the associated pass is done, instead of at the end of all the passes in `run_passes`. However, it looks like `-Zvalidate` is broken with current nightlies anyways 😢 (it spits out a bunch of errors). cc `@oli-obk` `@wesleywiser` r? rust-lang/wg-mir-opt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A more powerful version of #77665.
The goal is to allow fine-grained ordering constraints between passes and to make violating those constraints a compile-time error (and partly to motivate #90488 😏). The compile-time part would be very hard when enforcing constraints across queries, so we continue to rely on
MirPhasefor things like "optimizations run after drop elaboration". However, it works okay within a single invocation ofrun_passes(e.g. all the optimizations), where adding a few constraints to each pass is less of a pain than adding a bunch of variants toMirPhase. Since constraints are declared explicitly, we can do cool stuff like check that they hold for all possible opt-levels, -Z flags, etc.For now it's just a proof of concept, and a silly one at that. I don't know that we ultimately want to go in this direction, but it was pretty fun. It turns out writing nested
forloops in a const-context leads to some pretty ugly code. @oli-obk might be into it at least 😄. I only translated one small section of MIR passes so that people could get a feel for how it would look. We probably will want a different interface; the batch of associated constants is pretty unpleasant to look at IMO.All the interesting stuff is in the last few commits. The rest are refactorings or const-eval stuff to get everything working. Ideally, each pass would declare which cleanup passes it needs, but for that I'll need to move all the MIR passes back into
rustc_mir_transform.